Skip to content

fix-1392-Inconsistency-in-User-Verification-Status #2112

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

fix-1392-Inconsistency-in-User-Verification-Status #2112

wants to merge 3 commits into from

Conversation

MicahHeneveld
Copy link

What does this PR do?

Fixes this bug:

👍 Expected behavior
When the user hits Verify Phone, the alert should confirm that the user is verified. When the user hits Unverify Phone, the alert should confirm that the user is unverified. 👎 Actual Behavior
When attempting to verify the phone number, an alert incorrectly states that the user is unverified. However, when attempting to unverify the phone, the system incorrectly indicates that the user is verified.

Followed stnguyen90's guidelines:
We want to update the alerts to be specific to say:

XYZ's email has been verified
XYZ's email has been unverified
XYZ's phone has been verified
XYZ's phone has been unverified

Note:
The "s" should only be there if the name does not end with s. For example, if the user's name was "Charles", it should be "Charles' email has been verified"

Changed these lines:
message: ${$user.name || $user.email || $user.phone || 'The account'} has been ${ !$user.emailVerification ? 'unverified' : 'verified' },
message: ${$user.name || $user.email || $user.phone || 'The account'} has been ${ $user.phoneVerification ? 'unverified' : 'verified' },

To these lines:
message: ${$user.name || $user.email || $user.phone || 'The account'}${( $user.name || $user.email || $user.phone || 'The account' ).endsWith('s') ? "'" : "'s"} email has been ${!$user.emailVerification ? 'unverified' : 'verified'},

            message: `${$user.name || $user.email || $user.phone || 'The account'}${(
                $user.name || $user.email || $user.phone || 'The account'
            ).endsWith('s')
                ? "'"
                : "'s"} phone has been ${!$user.phoneVerification ? 'unverified' : 'verified'}`,

Current behavior:

XYZ's email has been verified
XYZ's email has been unverified
XYZ's phone has been verified
XYZ's phone has been unverified

OR (If the user's name ends in a s)

QRS' email has been verified
QRS' email has been unverified
QRS' phone has been verified
QRS' phone has been unverified

Test Plan

Under a project, select Auth. In Auth, select or create a user that has both an email and a phone number. Select the 'Verify account' option and then verify the phone number and email. The correct alerts will be displayed.

Have you read the Contributing Guidelines on issues?

Yes

@MicahHeneveld
Bug that was fixed:
6da4bf8 
👍 Expected behavior
When the user hits Verify Phone, the alert should confirm that the user is verified.
When the user hits Unverify Phone, the alert should confirm that the user is unverified.
👎 Actual Behavior
When attempting to verify the phone number, an alert incorrectly states that the user is unverified.
However, when attempting to unverify the phone, the system incorrectly indicates that the user is verified.

Followed stnguyen90's guidelines:
We want to update the alerts to be specific to say:

XYZ's email has been verified
XYZ's email has been unverified
XYZ's phone has been verified
XYZ's phone has been unverified

Note:
The "s" should only be there if the name does not end with s. For example, if the user's name was "Charles", it should be "Charles' email has been verified"

Changed these lines:
                 message: `${$user.name || $user.email || $user.phone || 'The account'} has been ${
                     !$user.emailVerification ? 'unverified' : 'verified'
                 }`,
                 message: `${$user.name || $user.email || $user.phone || 'The account'} has been ${
                     $user.phoneVerification ? 'unverified' : 'verified'
                 }`,

To these lines:
                message: `${$user.name || $user.email || $user.phone || 'The account'}${(
                    $user.name || $user.email || $user.phone || 'The account'
                ).endsWith('s')
                    ? "'"
                    : "'s"} email has been ${!$user.emailVerification ? 'unverified' : 'verified'}`,

                message: `${$user.name || $user.email || $user.phone || 'The account'}${(
                    $user.name || $user.email || $user.phone || 'The account'
                ).endsWith('s')
                    ? "'"
                    : "'s"} phone has been ${!$user.phoneVerification ? 'unverified' : 'verified'}`,

Current behavior:

XYZ's email has been verified
XYZ's email has been unverified
XYZ's phone has been verified
XYZ's phone has been unverified

OR (If the user's name ends in a s)

QRS' email has been verified
QRS' email has been unverified
QRS' phone has been verified
QRS' phone has been unverified
@MicahHeneveld
Copy link
Author

@stnguyen90 @ItzNotABug PR review request. A PR was submitted and reviewed by another contributor after mine was submitted. I don't know if you still want to review this one.

@MicahHeneveld MicahHeneveld mentioned this pull request Jul 14, 2025
Open
2 tasks
@Joyal-George-KJ
Copy link
Contributor

@stnguyen90 @ItzNotABug PR review request. A PR was submitted and reviewed by another contributor after mine was submitted. I don't know if you still want to review this one.

That was me that submitted the PR. Sorry I didn't see your PR under the issue so I thought you forgot/left the issue. I did the PR similar to yours and they said I had to make some changes and after I made the changes they said someone else was working on it. @MicahHeneveld you should check out my PR just to be clear on what they wanted in case yours is next. I added another Issues fix in the same PR too. You can ignore that!

Note

I didn't saw that an Issue has to be assigned before you proceed with fix in the contribution guideline that's why I made the fix.

@appwrite Appwrite
Copy link

appwrite bot commented Aug 12, 2025 •
edited
Loading

Console

Project ID: 688b7bf400350cbd60e9

Sites (2)
Site Status Logs Preview QR
 console-qa
688b7cf6003b1842c9dc		 Failed Failed Authorize Preview URL QR Code
 console-cloud
688b7c18002b9b871a8f		 Failed Failed Authorize Preview URL QR Code

Note

You can use Avatars API to generate QR code for any text or URLs.

@anthonythang1
Copy link

Hi, I'm working alongisde @MicahHeneveld! We improved the PR to fit the updated standards:

We want to update the alerts to be specific to say:

if name exists and isn't empty:

The email for XYZ has been verified
The email for XYZ is no longer verified
The phone for XYZ has been verified
The phone for XYZ is no longer verified

if name does not exist or is empty:

The email has been verified
The email is no longer verified
The phone has been verified
The phone is no longer verified

Here's what we changed:

            message: `${$user.name || $user.email || $user.phone || 'The account'}${(
                $user.name || $user.email || $user.phone || 'The account'
            ).endsWith('s')
                ? "'"
                : "'s"} email has been ${!$user.emailVerification ? 'unverified' : 'verified'}`,

To this:

        const hasName = $user.name && $user.name.trim() !== '';
        const isVerified = !$user.emailVerification;
        
        const message = hasName 
            ? `The email for ${$user.name} has been ${isVerified ? 'verified' : 'no longer verified'}`
            : `The email has been ${isVerified ? 'verified' : 'no longer verified'}`;

The code snippet is the same for the email and phone verification with some modifications to the variable names. This change allows the system to read if a name is provided and acts accordingly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ItzNotABug ItzNotABug Awaiting requested review from ItzNotABug

@stnguyen90 stnguyen90 Awaiting requested review from stnguyen90

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@MicahHeneveld @Joyal-George-KJ @anthonythang1